-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provider views rewrite (.files, .folders => .partialTree) #5050
Conversation
GoogleDrive - travelling down into folders works - checking a file works - breadcrumbs DONT work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
many great improvements in code quality and ux improvements in the file selector! seems to be working nicely. the code changes are huge, but sometimes big refactors are needed to move forward. as mentioned by others it's a bit hard to review in without first getting a deep understanding of the code, which will probably take a few hours/days (I don't have time for that now). but I trust you have tested this well and it seems to have good test coverage so it should be good to merge so people can start trying it out.
export type PartialTreeStatusFile = 'checked' | 'unchecked' | ||
export type PartialTreeStatus = PartialTreeStatusFile | 'partial' | ||
|
||
export type PartialTreeFile = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe id
, parentId
and data
can be pulled out into a common type, e.g. PartialTreeBase
and then PartialTreeFile
and PartialTreeFolderNode
can extend
from it? to reduce duplication and make it more clear which fields they have in common
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having PartialTreeBase
would add a level of indirection without making these types more readable/easier to work with.
In general - I avoided having many types here, because I want people who work on these files to largely remember existing types by heart (that's just 4 types - PartialTree
, ~File
, ~FolderNode
, ~FolderRoot
).
E.g., one really seductive type was PartialTreeDisplayedNode = ~File | ~FolderNode
(because these are the only nodes that can be seen in the interface, meaning most functions expect ~File | ~FolderNode
as an argument), but I decided against it to cut down on the number of types the developer would have to memorize.
…iew.tsx Co-authored-by: Mikael Finstad <finstaden@gmail.com>
…oadit/uppy into lakesare/provider-folders
…e quality, adds video icons)
Tested with
The testing process consists of the following procedure: |
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
| Package | Version | Package | Version | | ---------------------- | ------------- | ---------------------- | ------------- | | @uppy/audio | 2.0.0-beta.7 | @uppy/image-editor | 3.0.0-beta.6 | | @uppy/aws-s3 | 4.0.0-beta.8 | @uppy/instagram | 4.0.0-beta.7 | | @uppy/box | 3.0.0-beta.8 | @uppy/onedrive | 4.0.0-beta.8 | | @uppy/companion | 5.0.0-beta.11 | @uppy/provider-views | 4.0.0-beta.10 | | @uppy/companion-client | 4.0.0-beta.8 | @uppy/react | 4.0.0-beta.8 | | @uppy/core | 4.0.0-beta.11 | @uppy/screen-capture | 4.0.0-beta.6 | | @uppy/dashboard | 4.0.0-beta.11 | @uppy/transloadit | 4.0.0-beta.10 | | @uppy/drop-target | 3.0.0-beta.6 | @uppy/unsplash | 4.0.0-beta.8 | | @uppy/dropbox | 4.0.0-beta.9 | @uppy/url | 4.0.0-beta.8 | | @uppy/facebook | 4.0.0-beta.7 | @uppy/utils | 6.0.0-beta.9 | | @uppy/file-input | 4.0.0-beta.6 | @uppy/vue | 2.0.0-beta.4 | | @uppy/form | 4.0.0-beta.5 | @uppy/webcam | 4.0.0-beta.9 | | @uppy/golden-retriever | 4.0.0-beta.6 | @uppy/xhr-upload | 4.0.0-beta.7 | | @uppy/google-drive | 4.0.0-beta.1 | @uppy/zoom | 3.0.0-beta.7 | | @uppy/google-photos | 0.2.0-beta.2 | uppy | 4.0.0-beta.13 | - @uppy/companion: implement facebook app secret proof (Mikael Finstad / #5249) - @uppy/provider-views: `Loader.tsx` - delete the file (Evgenia Karunus / #5284) - @uppy/vue: fix passing of `props` (Antoine du Hamel / #5281) - @uppy/google-photos: fix various issues (Mikael Finstad / #5275) - @uppy/transloadit: fix strict type errors (Antoine du Hamel / #5271) - @uppy/transloadit: simplify plugin to always run a single assembly (Merlijn Vos / #5158) - meta: update Yarn version and npm deps (Antoine du Hamel / #5269) - docs: prettier: 3.2.5 -> 3.3.2 (Antoine du Hamel / #5270) - @uppy/provider-views: Provider views rewrite (.files, .folders => .partialTree) (Evgenia Karunus / #5050) - @uppy/react: TS strict mode (Merlijn Vos / #5258) - meta: simplify `build:ts` script (Antoine du Hamel / #5262)
Description
enables indeterminate checkmark states
[fixes Nested folder selection does not work correctly #4609]
enables folder caching
fixes the issue where Unsplash was only loading one page
[fixes Unsplash doesn't load further pages #5000]
reworks restrictions system
[fixes Fix logic with adding folders toast message #4414]
In particular aggregate restrictions are now shown in the footer:
absDirPath
andrelDirPath
are injected in a single place[fixes pull/4537#discussion_r1260683896]
nOfSelectedFiles
is as smart as it gets nowfixes the UI issue where shift-clicking files gets them highlighted:
fixes the way shift-clicking works in grid providers such as Instagram/Unpslash
[fixes Shift-clicking works chaotically with Instagram/Unsplash #5063]
makes the GoogleDrive's VIRTUAL_SHARED_DIR checkable
[see this discussion transloadit.slack.com/archives/C0FMW9PSB/p1714529071856209]
TODOs for next PRs
I'll work on "mifi's PR + but custom handling for smaller n of files" in further PRs.
Notes to reviewers
.env
in Slack - it's underway, but not yet fully ready.If you need access to a particular provider - message me, and I'll send you the credentials.
This PR spans 2.5k lines across 46 files.
I separated some PRs from this PR, however further separation will make it harder to review rather than easier, this is largely a holistic change.
I think a sane way to review this PR (apart from looking through "Files changed" on github) would be to look through the code in
/uppy/packages/@uppy/provider-views
folder locally in your editor - just read the files and see if they make sense/if something worries you. Most of these files were updated/rewritten.